Fix cross-tenant id collision clobbering memories/tasks rows - #4

Summary

memories.id and tasks.id are plain TEXT PRIMARY KEY (not composite with user_id), and on the default local/CLI path an id is content-derived or caller-supplied with no user salt. INSERT OR REPLACE resolves conflicts purely on that key, so a second tenant writing the same id silently deletes and replaces the first tenant's row — not a duplicate error, silent cross-tenant data loss. Reachable in shared-DB bucket shard mode (sharding.py), where multiple users share one SQLite file.

Four production write paths were affected, all now guarded the same way save_capture's capture_id_override already guards captures.id — check whether the id belongs to a different user, and only re-salt with user_id in that collision case, so the ordinary non-colliding path keeps ids byte-identical:

• _save_memory
• _save_task
• both loops in rebuild_index_from_vault (re-inserts from vault markdown frontmatter; the vault root is shared across users in bucket mode)

A fifth path, the portable-memory bundle import, was audited and is already safe — its id is derived with user_id baked in.

Anti-resurrection follow-up (88a2e41)

The guard as first written re-salted the id before the tombstone check, so a memory tombstoned under its plain derived id was missed once another tenant took that id: tenant A forgets "m1", tenant B later saves its own memory as "m1", and A's next resync salts away from B's row, finds no tombstone under the salted id, and silently resurrects the forgotten memory. Both the pre- and post-salt id are now checked. This gap was found independently on audit/prod-db-concurrency (343f0ca) and ported here.

Second-order collision: the salt itself could clobber a third tenant (fccfe69)

Re-salting once was not enough. The salted id can itself be owned by a third tenant, so the write would delete their row instead — the same bug one level down. Verified against a real database: tenant C holding stable_id("mem_", "tenant-b:collide_mem") was silently erased when tenant B collided with tenant A and salted into it.

The four inline single-hop guards are replaced by _tenant_unique_id(), which chains deterministically until the id is free — bounded, then raises rather than falling through and clobbering. The first hop is unchanged so ids already written stay stable, and the user_id != ? predicate ignores the caller's own row so repeated saves stay idempotent instead of chaining further and accumulating duplicates.

Forensics for databases already damaged (7ff8aac)

The guards prevent future clobbers but cannot un-lose data a deployment already destroyed, and there was no way to tell whether a given database was ever hit. backend/app/tenant_forensics.py is a read-only scan that answers exactly that:

python -m backend.app.tenant_forensics /path/to/index.sqlite

A clobber only rewrote the memories/tasks row itself — sibling rows carry their own user_id, and memory_events is append-only and was never rewritten, so a row whose user_id disagrees with its memory's owner is physical evidence the memory changed hands. Eight checks, each reported with its own confidence rather than summed into one score. Exits 1 on high-confidence evidence so it can gate a deploy check.

Detection is deliberately not gated on the surviving tenant count: a clobber that took over a tenant's only memory removes them from memories entirely, so that gate would report a total wipe as a harmless single-tenant database. Caught by testing the tool against a database produced by actually running the pre-fix code.

Why not a composite primary key (f57a62a)

The obvious fix — migrating memories.id to PRIMARY KEY(user_id, id) like entities — is wrong and breaks at runtime. SQLite requires an FK's parent columns to be a PRIMARY KEY or UNIQUE, and five single-column child FKs point at memories(id): memory_entities, memory_topics, memory_relations (both source_memory_id and target_memory_id), and memory_vec_map (whose memory_id is itself independently UNIQUE). Verified empirically:

OperationalError: foreign key mismatch - "memory_entities" referencing "memories"

The only escapes are rewriting every child table to carry user_id, or keeping a standalone UNIQUE index on memories.id — which re-imposes the exact global uniqueness the composite key was meant to relax. entities could go composite precisely because nothing FKs to entities(id). So the invariant this schema needs is "memory/task ids are globally unique," enforced on write. Documented at both table definitions so it isn't attempted again.

Test plan

• Regression tests added (backend/tests/test_sharding.py) in the deployment shape that matters: bucket mode with shard_count=1 routes both tenants into one SQLite file.
  • both tenants' memory and task rows survive a colliding write, each still owning its own content
  • retrieval stays isolated — neither tenant's search surfaces the other's memory
  • the collision salt does not resurrect a forgotten memory
  • a salted id already owned by a third tenant does not clobber them
  • 12 tenants piling onto one id all survive, with distinct ids
  • repeated saves under collision stay idempotent (no duplicate rows)
  • 8 forensics tests, including detection of a total tenant erasure
• Verified load-bearing, not just green. Reverting storage.py to pristine main leaves a single memories row owned by whichever tenant wrote last and returns an empty search for the other; removing the second tombstone check resurrects the forgotten memory as mem_2fa8523d0a70.
• Full suite: 2147 passed, 6 skipped, 416 subtests, 2 failed. Both remaining failures were confirmed pre-existing by running them against pristine trace-cortex/main: test_rerank_eval (missing model2vec dependency) and test_macos_ui_quality_contract.

One failure was caused by this change, and is fixed here (90e9ad7)

test_import_diff.py::test_import_diff_route was initially mis-reported as pre-existing. It is not — it passes on pristine main (30/30) and fails with the guard. ImportDiffFastAPIEndpointTests shares one process-wide store across test methods, and two tests seeded the literal id "mem_python" under two different user_ids; unittest runs methods alphabetically, so the second seed clobbered the first. That test only ever passed because of the bug this PR fixes. _seed's memory_id is now parameterized and the second test uses a distinct id. Same root cause and fix as integration/final (638c965).

Merge note: this should land first

audit/prod-db-concurrency also carries these guards plus its own concurrency work, but it is not a peer of this PR and cannot merge to main yet — it is built on the entire taste-exclude feature, which does not exist in main.

So: land this, then rebase audit/prod-db-concurrency onto updated main (~10 min; conflicts are limited to storage.py and database.py, and since both branches now carry identical anti-resurrection code that region resolves by taking either side — its guard commits become no-ops). The alternative leaves a live cross-tenant data-loss bug in main while this sits blocked behind an unrelated feature.

The anti-resurrection fix has also been ported to the other branches that carried the guards without it (integration/final, audit/prod-security, audit/prod-reliability, audit/prod-backend-arch), so no merge order can reintroduce the resurrection bug.

CI

The 3 failing checks are pre-existing — main fails the identical 3 jobs (Distribution site, Python checks, Security scan). The Python failure is assertIn("hasAppleSignInEntitlement", source) in the macOS UI contract test; the distribution failure is a missing release DMG. This PR introduces no new failures.